Parameter and pinout cleanup: take2 #90
Conversation
Signed-off-by: Mike Thompson <[email protected]>
Signed-off-by: Michael Thomposn <[email protected]>
| module uvmt_cv32_dut_wrap #(// DUT (riscv_core) parameters. | ||
| // https://github.com/openhwgroup/core-v-docs/blob/master/cores/cv32e40p/CV32E40P_and%20CV32E40_Features_Parameters.pdf | ||
| parameter N_EXT_PERF_COUNTERS = 1, // TODO: this is 0 in riscv_core, which is wrong | ||
| INSTR_RDATA_WIDTH = 128, |
There was a problem hiding this comment.
This is the wrong/old parameter list. In the updated cv32e40p we will only have the following parameters left: PULP_CLUSTER, FPU, PULP_ZFINX, DM_HALTADDRESS.
| .PULP_SECURE (PULP_SECURE), | ||
| .FPU (0) | ||
| riscv_core #( | ||
| .N_EXT_PERF_COUNTERS (N_EXT_PERF_COUNTERS), |
There was a problem hiding this comment.
Wrong parameter list (riscv_core now only has PULP_CLUSTER
FPU, PULP_ZFINX, DM_HALTADDRESS as parameters)
| @@ -156,7 +156,7 @@ interface uvmt_cv32_core_interrupts_if ( | |||
| output logic irq_software, // dut input | |||
| output logic irq_timer, // dut input | |||
| output logic irq_external, // dut input | |||
| output logic [15:0] irq_fast, // dut input | |||
| output logic [14:0] irq_fast, // dut input | |||
There was a problem hiding this comment.
This is correct; assignment below is wrong.
| @@ -167,7 +167,7 @@ interface uvmt_cv32_core_interrupts_if ( | |||
| irq_software = 1'b0; | |||
| irq_timer = 1'b0; | |||
| irq_external = 1'b0; | |||
| irq_fast = {15{1'b0}}; | |||
| irq_fast = {14{1'b0}}; | |||
There was a problem hiding this comment.
Should be {15{1'b0}};
There was a problem hiding this comment.
cv32/tb/uvmt_cv32/uvmt_cv32_dut_wrap.sv does not correspond to the updates I made originally. It is using the old (i.e. very long) riscv_core parameter list, which will not compile together with the updated riscv_core that I made in openhwgroup/cv32e40p#278
Also the updates I made in core-v-verif/cv32/tb/core/riscv_wrapper.sv are now missing (so that testbench will not compile either).
The updates I made in cv32/tb/core/tb_top.sv are critical as well and are missing now (without them we are wrongly simulating all test cases with the wrong 128-bit wide instruction interface).
The updates I made in cv32/tb/uvmt_cv32/uvmt_cv32_tb.sv are now missing. We would be simulating a non-supported configuration.
Same remark for cv32/tb/uvmt_cv32/uvmt_cv32_tb_ifs.sv (it does not include all required changes)
|
Yep, I am aware of all this @Silabs-ArjanB (I should have indicated that in my message at the head of this PR). The changes in place for this PR compile and run without warnings in the testbench code using the specific version of the RTL that will be cloned by the Makefiles (see Common.mk). There are still several to address in the RTL. I have tested this with dsim and xrun on the uvm environment and verilator, xrun and dsim on the core testbench. Its possible (likely!) that the Makefiles for Questa are not working as all of our Questa users seem to be off-line these days (for good and obvious reasons). Once openhwgroup/cv32e40p#278 is committed to the head of the cv32e40p master branch, I will work on another fork and issue a PR to update the testbench to be compatible with it. The points you bring up will be addressed at that time. I'd like to get these changes merged in before openhwgroup/cv32e40p#278 to minimize the effort to update the testbench and environment for those changes to the RTL. |
|
Okay, then only a minor fix is needed in cv32/tb/uvmt_cv32/uvmt_cv32_tb_ifs.sv I was thrown off by the remark that once this PR is approved you would kill #277 |
Signed-off-by: Mike Thompson <[email protected]>
Done!
Yeah, I wasn't clear. Sorry 'bout that. |
cv32e40p corev_dv fixes for illegal handler and debug program
Hi Arjan. This is directly related to #277. I believe it addresses all you issues in that pull-request, plus a few other items I've been meaning to clean up. It also updates the CV32E40P_HASH and FPNEW_HASH variables to point to the latest RTL (which Davide has indicated is good ready for verif to use).
If you approve this PR I will kill #277.